Skip to content

Remove unnecessary "Apply and Close" button from Properties dialog#4014

Merged
elsazac merged 1 commit into
eclipse-platform:masterfrom
elsazac:removeApplyandClose
Jun 1, 2026
Merged

Remove unnecessary "Apply and Close" button from Properties dialog#4014
elsazac merged 1 commit into
eclipse-platform:masterfrom
elsazac:removeApplyandClose

Conversation

@elsazac

@elsazac elsazac commented May 18, 2026

Copy link
Copy Markdown
Member

The Properties dialog opened from Installation Details → Properties does not have any option to edit or make any modifications to the properties. Despite this it still displays both Cancel and Apply and Close buttons.

Since no changes can be made in this dialog, the Apply and Close button is basically redundant here and performs the same action as Cancel. This could be confusing to the users hence this commit removes the unnecessary Apply and Close button and retain only the cancel option.

Fixes :eclipse-equinox/p2#1049

Before:
Screenshot 2026-05-18 at 4 53 37 PM

After:
Screenshot 2026-05-18 at 4 54 13 PM

@BeckerWdf

Copy link
Copy Markdown
Member

After:
Screenshot 2026-05-18 at 4 54 13 PM

Shown "Close" instead of "Cancel" would even be better from my point of view.

@github-actions

github-actions Bot commented May 18, 2026

Copy link
Copy Markdown
Contributor

Test Results

   864 files  ±0     864 suites  ±0   51m 45s ⏱️ -45s
 7 988 tests ±0   7 745 ✅ ±0  243 💤 ±0  0 ❌ ±0 
20 418 runs  ±0  19 763 ✅ ±0  655 💤 ±0  0 ❌ ±0 

Results for commit c8a2696. ± Comparison against base commit 1e6a7c9.

♻️ This comment has been updated with latest results.

@elsazac
elsazac force-pushed the removeApplyandClose branch from 408812a to 6651650 Compare May 18, 2026 13:26
@elsazac

elsazac commented May 18, 2026

Copy link
Copy Markdown
Member Author

This is the updated view with close option :

image

This commit retains only the cancel button and eliminates the Apply and
close button in the property dialog which is unnecessary in the current
context there. There is only a description provided in the property
dialog and there is no event that needs an apply option.

Fixes: eclipse-equinox/p2#1049
@elsazac
elsazac force-pushed the removeApplyandClose branch from 6651650 to c8a2696 Compare June 1, 2026 04:28
@elsazac

elsazac commented Jun 1, 2026

Copy link
Copy Markdown
Member Author

build looks good; merging this now;

@elsazac
elsazac merged commit 62c792e into eclipse-platform:master Jun 1, 2026
18 checks passed
@iloveeclipse

Copy link
Copy Markdown
Member

This introduces regression elsewhere!

Right click any project, see a property page for the project that now has no "Apply and Close" button, only separated "Apply" and "Close".
Before:

image

After:

image

Please revert the change or provide a fix that doesn't affect other components.

@iloveeclipse
iloveeclipse requested a review from Copilot June 15, 2026 17:53

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR aims to simplify the Properties dialog UI by removing the redundant “Apply and Close” button when the dialog is effectively read-only (as described for Installation Details → Properties), to avoid confusing users.

Changes:

  • Overrides the Properties dialog button bar to show a single Close button instead of the default Apply and Close / Cancel buttons.
  • Routes Close button handling through cancel semantics (cancelPressed()).
  • Overrides updateButtons() as a no-op to avoid relying on the default OK-button enablement logic.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +96 to +108
@Override
protected void buttonPressed(int buttonId) {
if (buttonId == IDialogConstants.CLOSE_ID) {
cancelPressed();
return;
}
super.buttonPressed(buttonId);
}

@Override
protected void createButtonsForButtonBar(Composite parent) {
createButton(parent, IDialogConstants.CLOSE_ID, IDialogConstants.CLOSE_LABEL, true);
}
Comment on lines +181 to +184
@Override
public void updateButtons() {
// This function is overridden to remove the Apply and close button
}
Comment on lines +96 to +103
@Override
protected void buttonPressed(int buttonId) {
if (buttonId == IDialogConstants.CLOSE_ID) {
cancelPressed();
return;
}
super.buttonPressed(buttonId);
}
@elsazac

elsazac commented Jun 15, 2026

Copy link
Copy Markdown
Member Author

I will revert this change for now;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants